Add WaveReadLaneAt tests
#349
Conversation
- we only want to disable the minimal set of tests
test/WaveOps/WaveReadLaneAt.64.test
Outdated
| ... | ||
| #--- end | ||
|
|
||
| # REQUIRES: Double, Int64 |
There was a problem hiding this comment.
@bogner, I know we were talking about needing to double up testing, but I'm curious for your thoughts on this one.
Combining double and int64 in the same test means we don't run this on devices that only support one or the other. While I think int64 is fairly universal at this point because 64-bit pointer math kinda requires it, there are a number of mobile architectures that don't support double (including Apple GPUs).
Does it make sense to combine 64-bit types or should we split them?
There was a problem hiding this comment.
Yeah, splitting these makes sense. I suspect as a rule of thumb any time we're REQUIREing two different features in these tests we should probably split them up for maximum test coverage.
| int NegValue = WaveReadLaneAt(InInt[TID.x], NegShiftIndex); | ||
| OutShift[TID.x] = PosValue + NegValue; | ||
|
|
||
| uint MixIndex = 0; |
There was a problem hiding this comment.
Should we also have a test where WaveReadLaneAt is called in one or more switch statement cases?
There was a problem hiding this comment.
We should be careful using switch statements in tests. They should be fine as long as no case labels have fall through. Even trivial fall through is UB in SPIRV.
| @@ -0,0 +1,181 @@ | |||
| #--- source.hlsl | |||
There was a problem hiding this comment.
Should there be bool tests for WaveReadLaneAt? I see that it supports bool in the HLSL headers
This PR implements testing for the WaveReadLaneAt intrinsic. It's an improved copy of @inbelic's work here: #295
Resolves: #144